refactor(commands): trim end of pipe-like output
#10952
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm unsure if I implemented the change in full completeness, as the exiting logic is kind of hard to understand. When trying the commands in the editor, this worked fine: let output = if let Some(output) = shell_output.as_ref() {
output.clone()
} else {
let fragment = range.slice(text);
match shell_impl(shell, cmd, pipe.then(|| fragment.into())) {
Ok(result) => {
if !pipe {
shell_output = Some(result.clone());
}
- result
+ Tendril::from(result.trim_end())
}
Err(err) => {
cx.editor.set_error(err.to_string());
return;
}
}
};But the test failed. When adding this, the test passed: let output = if let Some(output) = shell_output.as_ref() {
- output.clone()
+ Tendril::from(output.trim_end())
} else {
let fragment = range.slice(text);
match shell_impl(shell, cmd, pipe.then(|| fragment.into())) {
Ok(result) => {
if !pipe {
shell_output = Some(result.clone());
}
result
Tendril::from(result.trim_end())
}
Err(err) => {
cx.editor.set_error(err.to_string());
return;
}
}
};But still not sure if edit: Changed to this and it worked with tests passing: let output = if let Some(output) = shell_output.as_ref() {
output.clone()
} else {
let fragment = range.slice(text);
match shell_impl(shell, cmd, pipe.then(|| fragment.into())) {
Ok(result) => {
+ let result = Tendril::from(result.trim_end());
if !pipe {
shell_output = Some(result.clone());
}
result
}
Err(err) => {
cx.editor.set_error(err.to_string());
return;
}
}
}; |
the-mikedavis
approved these changes
Jun 14, 2024
mxxntype
pushed a commit
to mxxntype/helix
that referenced
this pull request
Aug 14, 2024
kyruzic
pushed a commit
to kyruzic/helix
that referenced
this pull request
Sep 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Current
pipe-like commands,pipe,insert-output, andappend-output, return directly whatstdouthad, which usually includes a newline. This can limit its efficiency in the editor as when someone would want to run the same command on many selections, the additional whitespace can become cumbersome to manually trim.This PR trims all whitespace from the end of the output. This was mainly chosen do to the simplicity of the change, but even when testing an iteration where only a single newline was removed, it felt more inline with expectations and so was chosen for the final change.
pipe:exabase16_theme.toml book Cargo.lock Cargo.toml CHANGELOG.md contrib default.nix docs Files flake.lock flake.nix grammars.nix helix-core helix-dap helix-event helix-loader helix-lsp helix-parsec helix-stdx helix-term helix-tui helix-vcs helix-view languages.toml LICENSE logo.svg logo_dark.svg logo_light.svg README.md runtime rust-toolchain.toml rustfmt.toml screenshot.png shell.nix target theme.toml xtask -Closes: #10912